Skip to content

Conversation

JoeWang1127
Copy link
Collaborator

@JoeWang1127 JoeWang1127 commented Mar 13, 2025

In this PR:

@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Mar 13, 2025
@JoeWang1127
Copy link
Collaborator Author

slf4j 1.x failed:

Error:  /home/runner/work/sdk-platform-java/sdk-platform-java/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/logging/ITLogging1x.java:[59,45] cannot access org.slf4j.spi.LoggingEventAware
  class file for org.slf4j.spi.LoggingEventAware not found

@@ -281,7 +281,7 @@
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.3.15</version>
<version>1.2.13</version>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restore the version to fix compilation error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious what was the compilation error? I did not came across when changing to 1.3.x here.
Also, can you keep logback-classic and logback-core versions consistent. Perhaps add a property for them

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compilation error is #3698 (comment).

I did not came across when changing to 1.3.x here.

I think the showcase test got removed before merging #3686.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. interesting, I might look into this error later.
For now, I agree lets use 1.2.x versions for both

@JoeWang1127 JoeWang1127 marked this pull request as ready for review March 13, 2025 21:15
@JoeWang1127 JoeWang1127 requested a review from zhumin8 March 13, 2025 21:15
Copy link
Contributor

@zhumin8 zhumin8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be a separate pr: on maintaining the testing versions for logback in gapic-showcase/pom.xml, I was thinking to setup renovate as in this pr. Any better ideas?

@@ -281,7 +281,7 @@
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.3.15</version>
<version>1.2.13</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious what was the compilation error? I did not came across when changing to 1.3.x here.
Also, can you keep logback-classic and logback-core versions consistent. Perhaps add a property for them

@JoeWang1127
Copy link
Collaborator Author

this could be a separate pr: on maintaining the testing versions for logback in gapic-showcase/pom.xml, I was thinking to setup renovate as in this pr. Any better ideas?

Maybe we can find a way to lock the logback version in slf4j 1.x testing but configure renovate to update others.

@JoeWang1127 JoeWang1127 requested a review from zhumin8 March 14, 2025 14:10
Copy link

@zhumin8
Copy link
Contributor

zhumin8 commented Mar 14, 2025

Maybe we can find a way to lock the logback version in slf4j 1.x testing but configure renovate to update others.

This is what I would prefer, but from quick search , I did not find renovate have feature to mark ignore.

Copy link

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@JoeWang1127 JoeWang1127 enabled auto-merge (squash) March 14, 2025 14:21
@JoeWang1127 JoeWang1127 merged commit 02c61cf into main Mar 14, 2025
52 of 53 checks passed
@JoeWang1127 JoeWang1127 deleted the chore/restore-showcase branch March 14, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants